fix(diffing): strip session-local sdBlockId from diff fingerprint (SD-3279)#3527
Conversation
…-3279) `editor.doc.diff.apply` checks that the editor it runs against has the same canonical fingerprint as the editor that produced the diff. Two SuperDoc editor instances loaded from the same DOCX assign different `sdBlockId` UUIDs at editor startup, so their canonical states diverge and the cross-editor handoff fails with PRECONDITION_FAILED. The attribute-diff path already treats `sdBlockId` as non-semantic (IGNORED_ATTRIBUTE_KEYS in attributes-diffing.ts), and the table-cell NO_OP comparator already lists `sdBlockId` + `sdBlockRev` + paraId + textId + rsid* as identity attrs. The bug was internal drift: the diff fingerprint normalization only stripped paraId / textId / rsid* from paragraph attrs and ignored sdBlockId / sdBlockRev entirely. Hoist the identity-attr list into a shared constant (`NON_SEMANTIC_BLOCK_ATTRS` in `extensions/diffing/algorithm/identity-attrs.ts`) and use it from both consumers. Apply the strip to every block node, not just paragraphs — `sdBlockId` lives on tables, rows, cells, and sections too. Tracked sdBlockRev as adjacent hardening (same identity-attr category, already in the table comparator's list); the diagnostic only observed sdBlockId divergence in the two-editor snapshot payloads. Tests: - Canonicalization: `normalizeDocJSON` now strips identity attrs from non-paragraph block nodes; two doc trees that differ only in sdBlockId values normalize to the same shape. - Fingerprint: `buildCanonicalDiffableState` produces the same fingerprint for body trees that differ only in identity attrs. - Cross-editor handoff: a diff produced by one editor instance applies cleanly to a second editor instance with the same content (the customer's preview-pane pattern from IT-1116). - Same-editor regression guard: capture / mutate / compare / apply on one editor still works. - Export → reimport regression marker: covers the second canonical divergence layer beyond sdBlockId. Currently asserts the known failure mode; flip to a success assertion when SD-3282 is fixed. Verified: - 252/252 diffing tests pass. - `pnpm check:types` clean. - 11 image-move conformance failures pre-existed on main; confirmed unrelated to this change by stashing and re-running.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Locks in the public Document API surface for the customer's preview-pane pattern (IT-1116). Three sessions, base and preview sharing identical content; target produces the snapshot; main session computes the diff; the diff is applied to the *separately-opened* preview session as tracked changes. Pre-SD-3279 this throws PRECONDITION_FAILED because the two sessions' canonical fingerprints diverge on session-local sdBlockId values. The service-level integration test in diff-service.test.ts already covers this; the story sits at the SDK/CLI dispatch tier that customers actually use, so a regression in the public path would be caught even if the internal helper kept passing. Verified end-to-end via vitest run against the rebuilt CLI + SDK on the fix branch.
The earlier story commit verified apply result + tracked changes total. Adds the customer-visible side: the preview session's body text now contains the target paragraph after the diff is applied. Same single test, one extra assertion. Catches a class of regression where the apply succeeds in tracking-changes accounting but never lands the underlying content.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d643338c2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
cubic analysis
No issues found across 7 files
Linked issue analysis
Linked issue: SD-3279: diff.apply can fingerprint-mismatch across editor instances loaded from the same DOCX
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Strip session-local sdBlockId (and sdBlockRev) from the diff fingerprint normalization so fingerprint does not vary across editor sessions. | A shared NON_SEMANTIC_BLOCK_ATTRS constant now includes sdBlockId and sdBlockRev and is used by the normalization code; unit tests assert normalized output/fingerprint is identical when only sdBlockId/sdBlockRev differ. |
| ✅ | Strip identity-only attributes from every block node (paragraphs, tables, rows, cells, sections) rather than only paragraphs so fingerprints are stable for all block types. | normalizeDocNodeJSON now applies block-attr normalization to container nodes and tests verify identity attrs are stripped from table/row/cell/section nodes. |
| ✅ | Unify the identity-attr list into one shared constant consumed by both the diff fingerprint normalization and the cell-content NO_OP comparator to prevent drift. | New NON_SEMANTIC_BLOCK_ATTRS constant is added and consumed from identity-attrs.ts; table adapter now imports that set instead of duplicating the list. |
| ✅ | Cross-editor handoff: a diff produced by one editor instance can be applied to a second editor instance (same DOCX content) in tracked-change mode and produces tracked-change operations. | The PR adds tests that exercise capture/compare/apply across separate editor instances and the PR description and tests indicate the cross-editor handoff test passes post-fix. |
| ✅ | Preserve same-editor capture/compare/apply behavior (regression guard). | Tests explicitly include a same-editor regression guard and the PR notes same-editor path still passes; test asserts tracked changes are applied and content matches target. |
The previous SD-3279 commit changed the canonical normalization (stripped sdBlockId / sdBlockRev) without bumping the snapshot/payload version. Any DiffSnapshot persisted by a customer under the pre-fix algorithm would now be rejected as tampered when re-derivation runs through the new normalizer. Same risk for DiffPayloads in same-session reapply. Add a parallel legacy normalization path that reproduces the pre-fix behavior exactly (paragraph-only attribute stripping over the original 7-element set, no stripping on structural containers, image originalAttributes stripping unchanged). compareToSnapshot and applyDiffPayload now accept either the new or the legacy fingerprint: re-derive with the current normalizer first; on mismatch, retry with the legacy normalizer. Genuinely tampered artifacts fail both checks and still throw. Scope of recovery: - DiffSnapshot persisted under old algorithm: fully recovered. This is the customer's revision-history workflow (capture and store). - DiffPayload persisted under old algorithm: recovered only for same-editor reapply where the editor still holds the original sdBlockIds. Cross-session reapply of old payloads was already broken by per-session sdBlockId divergence — that's the exact bug SD-3279 fixes for new snapshots; the legacy fallback can't retroactively rescue it. Tests added: - diff-service.test.ts: legacy snapshot accepted by compareToSnapshot; legacy same-editor payload accepted by applyDiffPayload; genuinely tampered snapshot still rejected. - fingerprint.test.ts: legacy normalizer produces a different fingerprint than the current normalizer for docs with sdBlockId (otherwise the fallback would be dead code). - diff-service.test.ts: SD-3282 ticket reference added to the existing export → reimport regression marker comment. Verified: 257/257 diffing tests pass; pnpm check:types clean.
Replay previously called setNodeMarkup with the diff's full source attrs, overwriting the recipient editor's session-local sdBlockId / sdBlockRev with the originator's values. Merge the AttributesDiff onto the recipient node's existing attrs so any path the diff does not touch is preserved verbatim. Also pass NON_SEMANTIC_BLOCK_ATTRS as ignoreKeys for non-paragraph block attr diffs; paragraphs were already covered via normalizeParagraphAttrs.
There was a problem hiding this comment.
cubic analysis
No issues found across 16 files
Linked issue analysis
Linked issue: SD-3279: diff.apply can fingerprint-mismatch across editor instances loaded from the same DOCX
| Status | Acceptance criteria | Notes |
|---|---|---|
| ✅ | Remove session-local sdBlockId/sdBlockRev (and related identity attrs) from the diff/fingerprint normalization for all block nodes (paragraphs, tables, rows, cells, sections). | Normalization was extended to strip the non-semantic block attrs from every block node and tests validate normalized outputs are stable across differing sdBlockId values. |
| ✅ | Hoist the identity-attr list into a single shared constant and consume it from both the fingerprint normalizer and NO_OP comparators to prevent drift. | A single NON_SEMANTIC_BLOCK_ATTRS constant was added and imported where needed; previous duplicated lists were removed/replaced. |
| ✅ | Ensure replay/apply does not overwrite the recipient editor's session-local identity attrs when applying attribute diffs (preserve recipient sdBlockId/sdBlockRev). | An applyAttrsDiff helper was implemented and replay paths now merge diffs onto the recipient node attrs instead of replacing them; unit tests assert recipient sdBlockId is preserved. |
| ✅ | Fix the customer-visible bug: a diff produced by one editor instance can be applied to a separately-opened editor instance loaded from the same DOCX (cross-editor capture → compare → apply works). | End-to-end tests exercising cross-editor handoff were added and assert that a diff captured from one editor can be applied to another editor with the same content; this is the primary reported failure and the test passes per PR verification. |
| ✅ | Preserve backward-compatibility: accept snapshots and same-editor diff payloads captured under the pre-fix algorithm via a legacy normalizer fallback (without changing forward path behavior). | Legacy normalization functions and a legacy canonical-state builder were added; compareToSnapshot and applyDiffPayload retry fingerprint validation with the legacy canonicalizer before rejecting; tests cover accepting legacy snapshots and same-editor legacy payloads and rejecting tampered snapshots. |
| ✅ | Add a regression marker for the known export → reimport fingerprint divergence (document as SD-3282) without changing scope of this PR. | A regression marker test was intentionally added expecting the export→reimport path to still throw a fingerprint mismatch; the test documents the known limitation and is present in the test suite. |
### Bug Fixes - copy-pasted text in suggestion mode (#3576) - center virtualized matches after mount in find nav (SD-3315) - stop find navigation jumping to the reverted caret (SD-3315) - honor the focus() contract + fix dangling docs reference (SD-3312) - don't re-center visible matches on find navigation (SD-3315) - report 'zoom' not 'mixed' for a zoom repaint (SD-3311) - wire pointer-source tracking on all init paths; update export snapshot - reset state on unload, dedupe + export payload types, add core tests - skip empty block SDT content selection - type modules.contentControls exactly (no pass-through index) - allow block SDT wrapper deletion to follow lock rules - promote image-bearing inline SDT wrappers to inline-block - keep block sdt fill behind content - hide block sdt fills in output modes - paint block SDT background on chrome layer - use logical inset for inline SDT label position - anchor inline SDT label to start of chrome - route smartTag through export and preserve smartTagPr in SDT flatten (SD-2647) - include inline SDT chrome width in block SDT bounds - keep block SDT chrome at full fragment width - render and round-trip w:smartTag content (SD-2647) - decouple base64 image helper imports - preserve block ids during metadata updates - ignore covered sdt label clicks - sync block sdt label selection updates - show empty SDT placeholder text in viewing and print modes - skip empty sdt scan on arrow right - preserve permission-only sdt placeholders - preserve comment-only sdt placeholders - preserve empty sdt bookmark placeholders - collapse hidden sdt placeholder text - keep sdt placeholder pm range atomic - trust empty sdt paragraph conversion - keep vanished sdt paragraph side effects - preserve vanished block sdt paragraphs - suppress hidden block sdt chrome - keep remeasured sdt placeholder atomic - transform sdt placeholder measure - remeasure sdt placeholders - hide sdt placeholders in print - hide sdt placeholders in viewing - ignore collapsed inline sdt cut - expose block sdt appearance - hide empty block sdt placeholder - align empty block sdt caret - use measured width for empty SDT placeholders - size SDT block labels to content width - collapse selection on sdtContentLocked delete - allow history transactions through sdt lock - drop unreachable move fallback - target marker-only textblock end - ignore empty block sdt key targets - cap block sdt label width - handle sdt marker gaps and block atoms - skip hidden field annotations in sdt navigation - skip hidden metadata sdt markers - skip hidden block sdt markers - handle marker-only sdt paragraphs - keep visible atoms in sdt navigation - skip hidden sdt navigation markers - target nearest sdt cursor position - respect inline atoms in sdt navigation - handle empty block sdt navigation - avoid restoring dragged block to its source position - exclude sdt chrome labels from caret position lookup - keep text-align enabled in locked SDT paragraphs - block disabled toolbar execution - guard unlisted locked toolbar commands - block locked sdt toolbar execution - reject malformed base64 image data URIs - reuse colliding data uri media targets - enforce upload byte cap for data uris - avoid non-image data uri extensions - reject raw raster data uri dimensions - validate oversized async svg images - register preset raster data uris in place - narrow sdt metadata overrides - normalize image data uri extensions - centralize image data url policy - reuse target image relationships - validate in-place svg payloads - validate field annotation data uri exports - reject malformed data uri files - avoid duplicate image rids - block raw raster data uri exports - reject malformed svg data uri payloads - warn on skipped image exports - reject separatorless data uri files - block non-image data uri exports - read svg data uri dimensions - share data uri media parsing - skip invalid data uri image targets - normalize svg data uri filenames - validate in-place svg image data - guard non-base64 data uri exports - export field annotation svgs as svg - allow non-base64 SVG data URLs in image rendering - mirror in-place image media to parent - decode non-base64 data URI exports - extract shared hash helpers - reuse data URI image exports - support non-base64 data URI images in registration - scope inline SDT placeholder to structuredContent metadata - register sized SVG data URI images without canvas processing - persist data URI images set via setPresetContent - share SDT lock predicates - version inline image metadata - align RTL SDT chrome to text - size SDT chrome for justified lines - honor ancestor image SDT locks - dirty inline image SDT changes - align SDT chrome within paragraph width - preserve SDT chrome continuation offsets - offset block SDT chrome for indents - suppress SDT pseudo hover in viewing mode - allow top-aligned inline images - fit block SDT chrome to actual content width - bottom-align text on lines with inline images - keep block SDT chrome and inline images out of paragraph geometry - disable image resize inside content-locked SDTs - detect inline image run changes in paragraph diff - mark block SDT selected when contained image is selected - select inline SDT content as text on Delete - intercept beforeinput insertText at inline SDT boundaries - delete contentLocked SDT wrapper in one step - bump sdBlockRev on ancestors of inline edits - select inline SDT content as text on Backspace - select inline SDT on Backspace at start of following run (SD-3165) - share structured content chrome label set - resolve block labels at node boundary - use contract label selectors - clear label gesture state on cancel - scope label clicks to owning editor - defer label selection to mouseup so native drag still fires - share structured content label classes - avoid deferred block label retry - select labels with active editor - focus editor after label selection - correct cursor placement and label interactions for structured content - handle cell-level SDT in vMerge column lookup (SD-3289) - preserve cell-level SDT wrapping table cells (SD-3289) - preserve recipient identity attrs on replay (SD-3279) - accept legacy fingerprints in compare/apply (SD-3279) - drop misleading pnpm run type-check hint from audit - structural-fail on missing dist + sync check-jsdoc header - strip session-local sdBlockId from diff fingerprint (SD-3279) - autoFit table width overflow from cell preferences (#3522) - flip public-method-coverage to strict-zero gate (SD-673) - flip jsdoc-hygiene-ts to strict-zero gate (SD-673) - rename jsdoc-hygiene-ts self-tests + wire into CI (fixes vitest discovery on #3511) - jsdoc-hygiene-ts handles private-identifier symbols + README - more bugs - add ui for overlapping delete, other fixes - replacement pair - remaining collab bugs - coalesce tracked inserts across run gaps - restore tracked change comment interactions - expose tracked mark predicate option - more cases - tc fixes - collab mode bug - floating comments fixes - make jsdoc-hygiene-ts baseline key line-independent + update wrapper docs - preserve tab underline via runProperties fallback in collab ### Changes - Merge branch 'main' into caio/sd-3315-find-replace-scroll - Merge pull request #3509 from superdoc-dev/artem/SD-3232 - Merge pull request #3555 from superdoc-dev/luccas/delete-image-content-locked-sdt - Merge branch 'main' into artem/SD-3159 - Merge pull request #3550 from superdoc-dev/luccas/sd-3302-bug-sdt-in-template-builder-shows-grey-background-highlight - Merge branch 'main' into artem/SD-3159 - SD-2676 - fix: table selection not providing a feedback (#3508) - Merge pull request #3549 from superdoc-dev/luccas/left-align-inline-sdt-label - Merge pull request #3546 from superdoc-dev/caio/sd-2647-bug-render-and-round-trip-content-wrapped-in-wsmarttag - Merge remote-tracking branch 'origin/stable' into sync/stable-to-main-20260527-230540 - Merge pull request #3539 from superdoc-dev/caio-pizzol/sd-3289-preserve-cell-level-sdt - Merge pull request #3527 from superdoc-dev/caio-pizzol/sd-3279-strip-sdblockid-from-diff-fingerprint - Merge branch 'main' into caio-pizzol/SD-js-contract-owner-audit - Merge pull request #3531 from superdoc-dev/caio-pizzol/SD-docs-snippet-typecheck-2 - Merge pull request #3526 from superdoc-dev/caio-pizzol/SD-runtime-payload-tests - Merge pull request #3521 from superdoc-dev/caio-pizzol/SD-public-method-coverage-strict-zero - Merge branch 'stable' - Merge pull request #3485 from superdoc-dev/artem/SD-3200 - Merge pull request #3517 from superdoc-dev/caio-pizzol/SD-jsdoc-hygiene-zero-flip - Merge pull request #3513 from superdoc-dev/caio-pizzol/SD-jsdoc-hygiene-cleanup - Merge branch 'stable' - Merge pull request #3511 from superdoc-dev/caio-pizzol/SD-jsdoc-hygiene-scanner - Merge pull request #3435 from superdoc-dev/nick/sd-3220-overlapping-suggestion-contract - Merge pull request #3429 from superdoc-dev/artem/underlined-tab-collab ### Documentation - note content controls in entityAt hit types (SD-3313) - clarify data uri buffer conversion - document image data uri helpers - clarify image registration comments - sync README exit semantics for js contract-owner audit - clarify wrapper-stages prose + add ts-jsdoc to check:public summary ### Features - add ui.contentControls.focus to place the caret in a control (SD-3312) - add ui.viewport.observe geometry-invalidation signal (SD-3311) - add ui.contentControls.scrollIntoView (SD-3310) - add activePath (full active stack) to content-control:active-change - expose public sdt events - select adjacent block SDT content at textblock boundaries - inherit run styles in empty block SDT placeholders - render placeholder text for empty SDTs - move caret into following block sdt on delete - move caret into preceding block sdt on backspace - disable mutation toolbar controls inside content-locked SDTs - render empty inline SDTs as a visible placeholder - add modules.contentControls.chrome - js contract-owner audit, report-only (SD-673) - add snippet typecheck for editor/superdoc/** + fix stale examples (SD-673) - add non-hover field color for sdt (#3506) - comments and tc on small screen (#3446) - overlapping tracked changes - add type-bearing JSDoc hygiene gate for .ts source (SD-673) - add anchored metadata orphan status ### Tests - resolve handleBase64 source path from package or repo root - cover locked block SDT Delete selection - cover nested block SDT boundary selection - update SDT keymap chain coverage - cover contentControlsChrome plumbing; clarify chrome-none comment - assert chrome-none hover suppression and cascade order - add v2 bridge unit + round-trip behavior coverage (SD-2647) - assert smartTag child text survives export round-trip (SD-2647) - cover nested block sdt navigation - roundtrip mixed image block sdts - cover image data uri length boundary - repaint saved sdt images through painter - cover structured content image edges - cover inline image diff fields - cover locked inline SDT beforeinput - cover inline SDT Cmd+X selection - cover inline SDT content Delete flow - clarify inline SDT boundary lock comment - cover two-step inline SDT Backspace - cover inline SDT selection meta escape - use real production payload for list-definitions-change bridge (SD-673) - tighten list-definitions-change bridge assertion (SD-673) - pin list-definitions-change bridge and DELETED comments shape (SD-673) - pin runtime event payload shapes (SD-673) ### Refactoring - consolidate type imports in dom painter - share block sdt navigation helpers - share box model between block and inline sdt labels - share structured content predicates - wrap shared tryDecodeDataUriText re-export - reuse shared data uri export policy - share image relationship export lookup - centralize image data uri parsing - share data uri text decoding - trim data uri metadata fields - extract CHECKED_FILES to shared module + drop audit from wrapper - drop list-item fragment renderer (SD-2851) (#3269) - drain 85 type-bearing JSDoc entries from .ts source (SD-673) ### Performance - avoid scanning data uri media ### Chores - 1.37.0 [skip ci] - fix import breaking delinstrtext orphans (#3535) - fix tests (#3533) - fixes - more fixes - run generate:all and fixes - more fixes - soec fixes - type fixes - ui and more - fix regression - type fixes - more fixes - more fixes - ci fixes - type fixes - add dispatch test for collab bug - review fix, type fix - add tests for metadata issue - tests for review issues
|
🎉 This PR is included in superdoc-cli v0.15.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.14.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.10.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.38.0 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/react v1.9.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.10.0 |
Two SuperDoc editor instances loaded from the same DOCX assign different
sdBlockIdUUIDs at editor startup, so the diff fingerprint differs across instances andeditor.doc.diff.applyfrom one editor to another throwsPRECONDITION_FAILED. This blocks the cross-editor preview pattern that customer revision-history UIs want to build (IT-1116).The attribute-diff path already treats
sdBlockIdas non-semantic (IGNORED_ATTRIBUTE_KEYS), and the table-cell NO_OP comparator already listssdBlockId/sdBlockRev/paraId/textId/rsid*as identity attrs. The bug was internal drift: the fingerprint normalization only stripped paragraph-levelparaId/textId/rsid*and never touchedsdBlockId/sdBlockRev, on paragraphs or anywhere else.NON_SEMANTIC_BLOCK_ATTRSinextensions/diffing/algorithm/identity-attrs.ts), consumed by both the diff fingerprint normalization and the table-cell NO_OP comparator. One source of truth so the lists cannot drift again.sdBlockIdlives on all of those.Backward compatibility for persisted artifacts
The normalization change would invalidate any DiffSnapshot or DiffPayload already persisted under the pre-SD-3279 algorithm (the customer's revision-history workflow stores DiffSnapshot blobs). To avoid breaking existing artifacts:
compareToSnapshotandapplyDiffPayloadre-derive with the current normalizer first, then fall back to a legacy normalizer that reproduces the pre-fix algorithm exactly. Either match accepts the artifact. Genuinely tampered snapshots fail both checks.sdBlockIdvalues are still intact; cross-session reapply of old payloads was already broken pre-fix and the fallback can't retroactively rescue it.Verified: 257/257 diffing tests pass.
pnpm check:typesclean. Cross-editor handoff test fails pre-fix withfingerprint mismatch, passes post-fix. Same-editor regression guard still passes. Legacy snapshot accepted bycompareToSnapshot; legacy same-editor diff payload accepted byapplyDiffPayload; genuinely tampered snapshot still rejected. The 11 image-move conformance failures incontract-conformance.test.tspre-existed on main; confirmed unrelated to this change. Added doc-api story covering cross-session diff handoff throughdoc.diff.capture/compare/apply.